Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kotlin: add cancel stream test and fix custom yaml path #1424

Merged
merged 11 commits into from
May 5, 2021

Conversation

buildbreaker
Copy link

@buildbreaker buildbreaker commented Apr 28, 2021

Adding a cancel stream test to mirror swift's cancel stream test: https://github.com/envoyproxy/envoy-mobile/blob/102b40d0da174200b4308a2677f6788a1df2a0aa/test/swift/integration/CancelStreamTest.swift

Found that the custom yaml configuration also has a bug in it where it does not appropriately register platform and native filters when a custom yaml is used. Luckily this is the first usage of the custom yaml. The JNI structure is different from the objective-c structure but I think I was able to mimic it as close as possible by just requiring an EnvoyConfiguration.

The API changes are here: https://github.com/envoyproxy/envoy-mobile/pull/1424/files#diff-c4d0f50e81c31efc078d029708e09a001ea3e466c6b38141c027fad21c6b70abR30-R34

Signed-off-by: Alan Chiu [email protected]

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description: kotlin: add cancel stream test and fix custom yaml path
Risk Level: low
Testing: integration test
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

@buildbreaker buildbreaker changed the title kotlin: add cancel stream test kotlin: add cancel stream test and fix custom yaml pathing Apr 28, 2021
@buildbreaker buildbreaker changed the title kotlin: add cancel stream test and fix custom yaml pathing kotlin: add cancel stream test and fix custom yaml path Apr 28, 2021
@buildbreaker buildbreaker marked this pull request as ready for review April 28, 2021 22:56
junr03
junr03 previously approved these changes May 4, 2021
Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, pending issue for missing method, and main merge. Thanks @buildbreaker! More tests, more better :)

Alan Chiu added 9 commits May 4, 2021 15:02
fix
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
fix
Signed-off-by: Alan Chiu <[email protected]>
Alan Chiu added 2 commits May 4, 2021 15:12
Signed-off-by: Alan Chiu <[email protected]>
Signed-off-by: Alan Chiu <[email protected]>
Copy link
Contributor

@goaway goaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, nice job @buildbreaker!

@buildbreaker buildbreaker merged commit 7c15bb6 into main May 5, 2021
@buildbreaker buildbreaker deleted the kotlin-add-cancel-stream-test branch May 5, 2021 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants